Add TypeScript definitions for Dialog component #509
Conversation
| export const theme: Object | ||
|
|
||
| export interface DialogProps extends CommonProps { | ||
| title: string |
There was a problem hiding this comment.
title can also be a React Node, but making it string | React.ReactNode causes an error since that's not compatible with the CommonProps title attribute.
To solve this I would need to change quite a bit the type definitions of CommonProps, BaseProps and probably the props of most of the components, let me know if it's worth it.
There was a problem hiding this comment.
@mxstbr do you know why title needs to be in CommonProps? https://github.com/primer/components/blob/master/index.d.ts#L9
There was a problem hiding this comment.
👋 Back from vacation now—probably because a tags can have a string title I would assume? That is copied from the rebass types if I remember correctly.
There was a problem hiding this comment.
@emplums I guess then that this PR should be fine to merge, and as a follow-up we can do the refactor of Props to support the title of a Dialog to be a React Node.
3 participants
This PR adds the TypeScript definitions for the
Dialogcomponent, which were missing.Merge checklist
index.d.ts) if necessary